Remove HF_TOKEN dependency in E2E test #357
Conversation
|
thank you for putting together this. My question is since the models and tokenizers are gated (and under certain term by meta), are they allowed to be saved in gcp bucket and distributed by us? |
Putting weights for our own use in e2e tests is fine, distributing weights and tokenizers publicly is not. |
| @@ -153,8 +155,13 @@ def _maybe_save_checkpoint(self, config: DictConfig) -> None: | |||
| # Step 3: Save the HF config files and tokenizer | |||
| if xr.process_index() == 0: | |||
| logger.info("Saving Hugging Face configs and tokenizer to %s", save_dir) | |||
| model_utils.copy_hf_config_files(config.model.pretrained_model, save_dir) | |||
| model_utils.save_hf_tokenizer(config.model.pretrained_model, save_dir) | |||
| # Copy to local if in GCS | |||
There was a problem hiding this comment.
Could you explain why is it necessary?
If training started from gcs fuse which looks like a local folder, would it still try to copy?
There was a problem hiding this comment.
This was needed because gcs bucket that we are loading toeknizer is not mounted by gcsfuse.
The bucket we mount in thunk.py is artifact_dir. The implementaion in this PR copies GCS content to local using gsutil instead of using gcsfuse.
| ) | ||
|
|
||
| local_dir = tempfile.mkdtemp() | ||
| _TEMP_DIRS_TO_CLEAN.append(local_dir) |
There was a problem hiding this comment.
This is a bad pattern, could you make temp dir as an argument or use context manager to auto clean it?
If that's inconvenient, feel free to leave it as is.
There was a problem hiding this comment.
Updated the function with context managager. Lmk if this look better.
…move-hf-toekn-e2e
Removing HF_TOKEN dependency in E2E test
tp save_hf_model_files_to_gcsto save huggingface model files in gcscopy_gcs_to_localwhich download in tmp directorytp save_hf_model_files_to_gcsexample:#14